FIX guard unsaved configuration changes - #2631
Open
Ching Wei Kang (WilliamK112) wants to merge 9 commits into
Open
Ching Wei Kang (WilliamK112) wants to merge 9 commits into
Ching Wei Kang (WilliamK112) wants to merge 9 commits into
Conversation
Signed-off-by: Ching-Wei Kang <ckang53@wisc.edu>
Roman Lutz (romanlutz)
requested changes
Sep 12, 2026
Roman Lutz (romanlutz)
left a comment
Roman Lutz (romanlutz)
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed a292338 from scratch. Both previous findings are addressed, and I resolved both threads:
- Failed environment-file reloads now preserve unsaved-change protection for retained drafts, including after further editing.
- The router now initializes after authentication restores the requested URL, so the rendered route matches the deep link.
I found no new confirmed production defects.
One caveat remains: some confirmation-dialog tests fail intermittently in JSDOM. In the diagnostic runs, the guard stayed active and the dialog was mounted, but it was marked aria-hidden. I reproduced the same failure with the unchanged shared dialog and tab controls, without this PR's guard or router code. I am therefore not treating it as a new blocking finding against this PR, but the test-reliability issue remains unresolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
beforeunloadguard for refresh, close, and hard navigationThis builds on the URL-backed Configuration tabs merged in #2606 and is updated onto the current
mainbranch.Closes #2594.
Tests and Documentation
npm test -- --runInBand src/components/Configuration/Configuration.test.tsx src/components/Configuration/EnvironmentFiles.test.tsx— 20 passednpm run type-check— passednpm run lint— passednpm run build— passedLabelsBarclick-away test failed both in the full run and in isolation (that component is outside this PR diff)No documentation changes are needed for this data-loss prevention fix. JupyText was not run because the change is frontend-only.
AI assistance
I used OpenAI Codex to help implement the navigation guards, update tests, merge current
main, and run validation. I reviewed the final diff and test results.